test: try to fix test-cli-node-options #14195

refack · 2017-07-12T16:28:19Z

partitioning the sub-process groups
use -e instead of module
reduce maxBuffer

Fixes: #14191

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

test,process,cli

refack · 2017-07-12T16:29:42Z

Pi CI: https://ci.nodejs.org/job/node-test-commit-arm-fanned/9924/

refack · 2017-07-12T16:30:04Z

/cc @nodejs/platform-arm @nodejs/testing @sam-github

refack · 2017-07-12T16:35:22Z

Anecdotal number from my local Windows machine:

D:\code\node>time < nul&D:\bin\dev\node\node9pre.exe D:\code\node\test\parallel\test-cli-node-options.js&time<nul
The current time is: 12:31:15.20
Enter the new time: The current time is: 12:31:19.92
Enter the new time:
D:\code\node>git checkout master
Switched to branch 'master'
Your branch is ahead of 'refack/master' by 4 commits.
  (use "git push" to publish your local commits)

D:\code\node>time < nul&D:\bin\dev\node\node9pre.exe D:\code\node\test\parallel\test-cli-node-options.js&time<nul
The current time is: 12:31:28.26
Enter the new time: The current time is: 12:31:34.14
Enter the new time:
D:\code\node>

After PR ~4.72s
Pre PR ~5.88s
Down to ~80% time (a.k.a 25% improvement 🤓)

richardlau · 2017-07-12T16:50:36Z

test/parallel/test-cli-node-options.js

-    maxBuffer: 1000000000,
-  };
-  exec(process.execPath, argv, opts, common.mustCall(function(err, stdout) {
+  const argv = ['-e', 'console.log("B")'];


If all we're doing with -e is console.log() perhaps consider using -p instead?

refack · 2017-07-12T17:11:05Z

-p 5.17s, 5.16s, 5.11s
-e 4.92s, 4.78s, 4.76

(faster by 5% with p = 2.3E-05)
Going back 🤷‍♂️

refack · 2017-07-12T19:27:05Z

https://ci.nodejs.org/job/node-test-binary-arm/9184/RUN_SUBSET=2,label=pi1-raspbian-wheezy/console

ok 30 parallel/test-cli-node-options-disallowed
  ---
  duration_ms: 9.884
  ...

https://ci.nodejs.org/job/node-test-binary-arm/9184/RUN_SUBSET=3,label=pi1-raspbian-wheezy/console

ok 29 parallel/test-cli-node-options
  ---
  duration_ms: 149.423
  ...

refack · 2017-07-12T19:27:30Z

irrelevant

@nodejs/testing
Ready for expedited landing

BridgeAR · 2017-08-30T21:22:43Z

Needs a rebase

refack · 2017-09-03T22:33:42Z

This is mainly to avoid timing out, currently on pi1-raspbian-wheezy the joined file can take > 40s

ok 31 parallel/test-cli-node-options
  ---
  duration_ms: 40.833
  ...

(https://ci.nodejs.org/job/node-test-binary-arm/RUN_SUBSET=1,label=pi1-raspbian-wheezy/9971)

split files take:

ok 30 parallel/test-cli-node-options
  ---
  duration_ms: 32.395
  ...

ok 31 parallel/test-cli-node-options-disallowed
  ---
  duration_ms: 5.89
  ...

/cc @nodejs/testing

Trott

LGTM if CI is green.

refack · 2017-09-04T12:10:50Z

CI: https://ci.nodejs.org/job/node-test-commit/12170/

* partitioning the subprocess groups * use `-e` instead of module * reduce maxBuffer PR-URL: nodejs#14195 Fixes: nodejs#14191 Reviewed-By: Rich Trott <rtrott@gmail.com>

* partitioning the subprocess groups * use `-e` instead of module * reduce maxBuffer PR-URL: nodejs/node#14195 Fixes: nodejs/node#14191 Reviewed-By: Rich Trott <rtrott@gmail.com>

* partitioning the subprocess groups * use `-e` instead of module * reduce maxBuffer PR-URL: #14195 Fixes: #14191 Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins · 2017-09-20T20:52:58Z

This does not land cleanly in LTS. Please feel free to manually backport by following the guide. Please also feel free to replace do-not-land if it is being backported

nodejs-github-bot added the test Issues and PRs related to the tests. label Jul 12, 2017

refack added flaky-test Issues and PRs related to the tests with unstable failures on the CI. cli Issues and PRs related to the Node.js command line interface. process Issues and PRs related to the process subsystem. arm Issues and PRs related to the ARM platform. labels Jul 12, 2017

refack requested a review from sam-github July 12, 2017 16:30

richardlau reviewed Jul 12, 2017

View reviewed changes

refack force-pushed the tweak-not-test-cli-node-options branch from 31efaf7 to e87ba7e Compare July 12, 2017 17:15

refack force-pushed the tweak-not-test-cli-node-options branch from e87ba7e to b6bb016 Compare September 3, 2017 21:44

Trott approved these changes Sep 4, 2017

View reviewed changes

test: split test-cli-node-options

86e7c61

* partitioning the subprocess groups * use `-e` instead of module * reduce maxBuffer PR-URL: nodejs#14195 Fixes: nodejs#14191 Reviewed-By: Rich Trott <rtrott@gmail.com>

refack force-pushed the tweak-not-test-cli-node-options branch from b6bb016 to 86e7c61 Compare September 4, 2017 21:52

refack merged commit 86e7c61 into nodejs:master Sep 4, 2017

refack mentioned this pull request Sep 8, 2017

test,flaky: test-cli-node-options #15284

Closed

MylesBorins pushed a commit that referenced this pull request Sep 10, 2017

test: split test-cli-node-options

5c7731a

* partitioning the subprocess groups * use `-e` instead of module * reduce maxBuffer PR-URL: #14195 Fixes: #14191 Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins mentioned this pull request Sep 10, 2017

v8.5.0 proposal #15308

Merged

MylesBorins pushed a commit that referenced this pull request Sep 11, 2017

test: split test-cli-node-options

df1f462

* partitioning the subprocess groups * use `-e` instead of module * reduce maxBuffer PR-URL: #14195 Fixes: #14191 Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins pushed a commit that referenced this pull request Sep 12, 2017

test: split test-cli-node-options

e3d0ff9

* partitioning the subprocess groups * use `-e` instead of module * reduce maxBuffer PR-URL: #14195 Fixes: #14191 Reviewed-By: Rich Trott <rtrott@gmail.com>

refack deleted the tweak-not-test-cli-node-options branch September 18, 2017 01:02

MylesBorins added the dont-land-on-v6.x label Sep 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: try to fix test-cli-node-options #14195

test: try to fix test-cli-node-options #14195

refack commented Jul 12, 2017

refack commented Jul 12, 2017

refack commented Jul 12, 2017

refack commented Jul 12, 2017

richardlau Jul 12, 2017

refack Jul 12, 2017

refack commented Jul 12, 2017 •

edited

Loading

refack commented Jul 12, 2017 •

edited

Loading

refack commented Jul 12, 2017 •

edited

Loading

BridgeAR commented Aug 30, 2017

refack commented Sep 3, 2017

Trott left a comment

refack commented Sep 4, 2017

MylesBorins commented Sep 20, 2017

test: try to fix test-cli-node-options #14195

test: try to fix test-cli-node-options #14195

Conversation

refack commented Jul 12, 2017

Checklist

Affected core subsystem(s)

refack commented Jul 12, 2017

refack commented Jul 12, 2017

refack commented Jul 12, 2017

richardlau Jul 12, 2017

Choose a reason for hiding this comment

refack Jul 12, 2017

Choose a reason for hiding this comment

refack commented Jul 12, 2017 • edited Loading

refack commented Jul 12, 2017 • edited Loading

refack commented Jul 12, 2017 • edited Loading

BridgeAR commented Aug 30, 2017

refack commented Sep 3, 2017

Trott left a comment

Choose a reason for hiding this comment

refack commented Sep 4, 2017

MylesBorins commented Sep 20, 2017

refack commented Jul 12, 2017 •

edited

Loading

refack commented Jul 12, 2017 •

edited

Loading

refack commented Jul 12, 2017 •

edited

Loading